Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Further improve README.md: point to specific sections of handbook, remove lengthy dandi ls --help example #1193

Merged
merged 2 commits into from
Jan 30, 2023
Merged

Further improve README.md: point to specific sections of handbook, remove lengthy dandi ls --help example #1193

merged 2 commits into from
Jan 30, 2023

Conversation

melster1010
Copy link
Collaborator

address 3 items outstanding from review in PR #1170

@codecov
Copy link

codecov bot commented Jan 26, 2023 •
edited
Loading

Codecov Report

Base: 89.09% // Head: 89.09% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (b71f647) compared to base (10921e3).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1193   +/-   ##
=======================================
  Coverage   89.09%   89.09%           
=======================================
  Files          76       76           
  Lines        9451     9453    +2     
=======================================
+ Hits         8420     8422    +2     
  Misses       1031     1031
Flag Coverage Δ
unittests 89.09% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dandi/utils.py 82.42% <0.00%> (+0.10%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yarikoptic
Copy link
Member

well, although I agree that sample output of --help commands can get outdated, I think I do like seeing them in README.md since it gives an immediate overview of exposed by the client functionality. If it is outdated already -- then updates are welcome for those parts.

@melster1010
Copy link
Collaborator Author

well, although I agree that sample output of --help commands can get outdated, I think I do like seeing them in README.md since it gives an immediate overview of exposed by the client functionality. If it is outdated already -- then updates are welcome for those parts.

After removing all of the UI graphics it does seem empty, and I understand that it would be useful to see something in the readme prior to using the tool...so I restored the first UI graphic of the initial interface but left out the other one for the help output, so a compromise! :)

waxlamp
waxlamp reviewed Jan 27, 2023
View reviewed changes
Copy link
Member

@waxlamp waxlamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove the listing of the dandi CLI help output and perhaps replace it with a few examples of common use cases for the tool. Let me know what you think of that.

@melster1010
Copy link
Collaborator Author

I think we should remove the listing of the dandi CLI help output and perhaps replace it with a few examples of common use cases for the tool. Let me know what you think of that.

I like that suggestion a lot, showing the UI and a couple of examples as done here. Which commands would be best to showcase, download and upload perhaps?

@yarikoptic yarikoptic mentioned this pull request Jan 30, 2023
Open
@yarikoptic
Copy link
Member

Let's proceed with this PR as is for now, as for improving README.md going forward with "demos", filed #1195 where we could continue.

@yarikoptic yarikoptic changed the title Fix outstanding readme issues from pr #1170 Further improve README.md: point to specific sections of handbook, remove lengthy dandi ls --help example Jan 30, 2023
@yarikoptic
Copy link
Member

Attn @melster1010 -- I changed the PR description to summarize changes since it would become a part of the changelog, so should be aiming at the user/worthy appearing in changelog.

@yarikoptic yarikoptic added the documentation Changes only affect the documentation label Jan 30, 2023
@yarikoptic yarikoptic merged commit 2edf051 into master Jan 30, 2023
@yarikoptic yarikoptic deleted the readme-cli-cleanup branch January 30, 2023 16:41
@github-actions
Copy link

🚀 PR was released in 0.49.0 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waxlamp waxlamp waxlamp left review comments

Assignees
No one assigned
Labels
documentation Changes only affect the documentation released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@melster1010 @yarikoptic @waxlamp